-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add selinux-vs-dmz test case and a workaround #4053
Conversation
5b9960c
to
60f5203
Compare
There has a discusstion in here: #3987 (comment), so maybe we should auto detect |
I think we can make the selinux case work, even if it involves changes to https://github.com/containers/container-selinux/ |
👍 Thanks |
60f5203
to
dd8a829
Compare
Something like this, yeah. PR updated. |
dd8a829
to
67c58b7
Compare
dbca1b5
to
34e12ee
Compare
59fe2b9
to
4dc8c38
Compare
fa1882c
to
ed230b6
Compare
runc_dmz_selinux_compat
build tag
ed230b6
to
0a860b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean the binary which we publish, sure, we can enable that workaround in there as well (see the last commit). |
cdb537d
to
256d89f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, the commits LGTM but the PR title and the description are out of sync with the actual commits
runc_dmz_selinux_compat
build tagThis is a test case to demonstrate the selinux vs dmz issue. The issue is, runc calls selinux.SetExecLabel and then execs the runc-dmz binary, but the execve is denied by selinux: > type=PROCTITLE msg=audit(10/05/2023 22:54:07.911:10904) : proctitle=/tmp/bats-run-sGk2sn/runc.Ql243q/bundle/runc init > type=SYSCALL msg=audit(10/05/2023 22:54:07.911:10904) : arch=x86_64 syscall=execveat success=no exit=EACCES(Permission denied) a0=0x6 a1=0xc0000b90fa a2=0xc0000a26a0 a3=0xc000024660 items=0 ppid=105316 pid=105327 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=pts0 ses=8 comm=runc:[2:INIT] exe=/tmp/bats-run-sGk2sn/runc.Ql243q/bundle/runc subj=unconfined_u:unconfined_r:container_runtime_t:s0-s0:c0.c1023 key=(null) > type=AVC msg=audit(10/05/2023 22:54:07.911:10904) : avc: denied { entrypoint } for pid=105327 comm=runc:[2:INIT] path=/memfd:runc_cloned:runc-dmz (deleted) dev="tmpfs" ino=2341 scontext=system_u:system_r:container_t:s0:c4,c5 tcontext=unconfined_u:object_r:container_runtime_tmpfs_t:s0 tclass=file permissive=0 Once that error is fixed (by adding a selinux rule that enables it), we see one more error, also related to executing a file on tmpfs. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add a workaround for a problem of older container-selinux not allowing runc to use dmz feature. If runc sees that SELinux is in enforced mode and the container's SELinux label is set, it disables dmz. Add a build tag, runc_dmz_selinux_nocompat, which disables the workaround. Newer distros that ship container-selinux >= 2.224.0 (currently CentOS Stream 8 and 9, RHEL 8 and 9, and Fedora 38+) may build runc with this build tag set to benefit from dmz working with SELinux. Document the build tag in the top-level and libct/dmz READMEs. Use the build tag in our CI builds for CentOS Stream 9 and Fedora 38, as they already has container-selinux 2.224.0 available in updates. Add a TODO to use the build tag for CentOS Stream 8 once it has container-selinux updated. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
fixed |
256d89f
to
87bd784
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though I'm not sure that we need runc_dmz_selinux_nocompat
either.
This is for distro packagers -- if they know they have a recent version of container-selinux, they can set this so that their users can enjoy both dmz and selinux. Once we drop CenOS 7, we can remove this flag (make this behavior a default). |
Now opencontainers/runc#4053 has fixed, Let's use the main branch of runc Signed-off-by: Sohan Kunkerkar <sohank2602@gmail.com>
Now opencontainers/runc#4053 has fixed, Let's use the main branch of runc Signed-off-by: Sohan Kunkerkar <sohank2602@gmail.com>
Now opencontainers/runc#4053 has fixed, Let's use the main branch of runc Signed-off-by: Sohan Kunkerkar <sohank2602@gmail.com>
Unfortunately, runc-dmz does not play well with SELinux.
The preliminary discussion about that happened in #3987 (comment), and as a result the
RUNC_DMZ=legacy
was introduced to disable the feature. Alas, the solution is not very practical, as users have to modify upper level runtimes to pass the env var to runc. Also, this is all-or-nothing approach.The issue itself is described in #4057, and is fixed in container-selinux v2.224.0.
For older distros, this PR adds a workaround (enabled by default). Here's a quote from libct/dmz/README (as added by this PR):
SELinux compatibility issue and a workaround
Older SELinux policy can prevent runc to execute the dmz binary. The issue is
fixed in container-selinux v2.224.0. Yet, some older distributions may not
have the fix, so runc has a runtime workaround of disabling dmz if it finds
that SELinux is in enforced mode and the container SELinux label is set.
Distributions that have a sufficiently new container-selinux can disable the
workaround by building runc with the
runc_dmz_selinux_nocompat
build flag,essentially allowing dmz to be used together with SELinux.